-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[plex] Initial contribution #12135
[plex] Initial contribution #12135
Conversation
5052031
to
0f652ac
Compare
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/plex-binding-for-oh3-will-it-be-ported/111487/178 |
Can somebody help/point me solving the build failures? It seems to fail, but not on the plex part. |
Maybe due to the full copyright you added in comments in the file feature.xml? |
Thanks, I will have a look. Is there any way to reproduce this locally, to avoid having to commit and wait each time... |
Yes, by running mvn clean install at the root of your git repo. |
3686846
to
c4c2e4c
Compare
Thank you @lolodomo . Managed to get it passing the build checks! |
This is my first contribution / pr so I'm not quite familiar with the way of working. Do I need to do anything to get this merged? |
6bc2d8b
to
75b50eb
Compare
Hi all, it has been a while, but letting you all know i am working on an update compatible for version 4.0 |
5ea9694
to
edf9baa
Compare
@fwolter thanks for the feedback. I've updated the PR as much as possible. Please have second look at it. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/plex-binding-for-oh3-stuck/136598/13 |
bundles/org.openhab.binding.plex/src/main/resources/OH-INF/binding/binding.xml
Outdated
Show resolved
Hide resolved
....binding.plex/src/main/java/org/openhab/binding/plex/internal/handler/PlexServerHandler.java
Outdated
Show resolved
Hide resolved
1775f74
to
808a603
Compare
Signed-off-by: Aron Beurskens <aron@beurskens.nl>
I have added "awaiting feedback" because there is still an unresolved comment for quite some time: #12135 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html
after building it with mvn clean install
.
There are some compiler warnings about null access and dead code left. You could take a look at mvn clean install
. To fix the null warnings, you need to store the field to a local variable and do the null check on that.
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[114,17] The method setupDefaultSecurity(com.thoughtworks.xstream.XStream) from the type com.thoughtworks.xstream.XStream is deprecated
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[162,17] Redundant null check: The variable user cannot be null at this location
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[166,20] Dead code
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[226,16] Null type mismatch (type annotations): 'null' is not compatible to the free type variable 'T'
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[284,28] The constructor org.eclipse.jetty.websocket.client.WebSocketClient(org.eclipse.jetty.util.ssl.SslContextFactory) is deprecated
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[335,25] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexApiConnector.java:[357,39] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexServerHandler.java:[56,54] The value of the field org.openhab.binding.plex.internal.handler.PlexServerHandler.stateDescriptionProvider is not used
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexServerHandler.java:[159,44] Potential null pointer access: The variable tmpMeta may be null at this location
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexServerHandler.java:[268,35] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexServerHandler.java:[301,36] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexServerHandler.java:[302,13] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\PlexHandlerFactory.java:[76,17] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexPlayerHandler.java:[48,30] The value of the field org.openhab.binding.plex.internal.handler.PlexPlayerHandler.plexAPIConnector is not used
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexPlayerHandler.java:[70,49] Potential null pointer access: The method getBridge() may return null
[WARNING] C:\Users\Fabian Wolter\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.plex\src\main\java\org\openhab\binding\plex\internal\handler\PlexPlayerHandler.java:[85,45] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] Code Analysis Tool has found:
0 error(s)!
8 warning(s)
11 info(s)
[WARNING] .binding.plex\README.md:[6]
The line before a Markdown list must be empty.
[WARNING] org.openhab.binding.plex.discovery.PlexDiscoveryService.java:[27]
First javadoc author should have "Initial contribution" contribution description.
[WARNING] org.openhab.binding.plex.discovery.PlexDiscoveryService.java:[29]
Classes/Interfaces/Enums should be annotated with @NonNullByDefault
[WARNING] org.openhab.binding.plex.internal.dto.MediaContainer.java:[23]
First javadoc author should have "Initial contribution" contribution description.
[WARNING] org.openhab.binding.plex.internal.handler.PlexApiConnector.java:[103]
Bei einem equals()-Vergleich sollten String-Literale auf der linken Seite stehen.
[WARNING] org.openhab.binding.plex.internal.handler.PlexPlayerHandler.java:[26]
Stern-Importe der Form '.*' sollten vermieden werden - org.openhab.core.thing.*.
[WARNING] org.openhab.binding.plex.internal.handler.PlexApiConnectorTest.java:[31]
First javadoc author should have "Initial contribution" contribution description.
[WARNING] org.openhab.binding.plex.internal.handler.PlexApiConnectorTest.java:[33]
Classes/Interfaces/Enums should be annotated with @NonNullByDefault
PlexPlayerConfiguration config = getConfigAs(PlexPlayerConfiguration.class); | ||
foundInSession = false; | ||
playerID = config.playerID; | ||
logger.warn("Initializing PLEX player : {}", playerID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.warn("Initializing PLEX player : {}", playerID); |
private final Map<String, PlexPlayerHandler> playerHandlers = new ConcurrentHashMap<>(); | ||
|
||
private PlexServerConfiguration config = new PlexServerConfiguration(); | ||
// private @Nullable PlexServerHandler bridge; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// private @Nullable PlexServerHandler bridge; |
</channel-type> | ||
<channel-type id="player"> | ||
<item-type>Player</item-type> | ||
<label>Player Control Play/Pause/Next/Previous</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Labels are expected to be as short as possible. Guideline is 2-3 words with max 20-25 chars. See https://www.openhab.org/docs/developer/bindings/thing-xml.html#formatting-labels-and-descriptions
<label>Player Control Play/Pause/Next/Previous</label> | |
<label>Player Control</label> |
How long does this remain in |
@aronbeurskens - would that be okay with you? And @fwolter, do you think you would be able to finish the review if @mlobstein would help resolving the last comments? |
Yes! |
Of course, no problem. When merged its also easier for me to maintain |
replaced by #15057 |
New binding: Plex
Description
This adds a new binding for communication with Plex.
It allows multiple players to be added to OpenHab and provide information about what is currently playing on the play.
When using this binding one can:
Related links
#6179
#4949
https://community.openhab.org/t/plex-binding-for-oh3-will-it-be-ported/111487
Testing
Latest .jar file available add: https://github.com/aronbeurskens/openhab-addons/releases/tag/v0.13